Substitute lodash calls in bitcore-lib #3350

picnicbear · 2022-03-24T14:12:05Z

No description provided.

… of _.sortBy, fix multiple issues resulting from regex replacements

…convert more forEaches, fix a reduce

…re-lib/removeLodash

…efined type checks

kajoseph

Nice job! I left a few suggestions, mostly for relatively minor quality of life and consistency/cleanliness changes. Feel free to ping me to discuss anything!

kajoseph · 2022-05-02T16:25:23Z

packages/bitcore-lib/lib/block/blockheader.js

@@ -69,10 +68,10 @@ BlockHeader._fromObject = function _fromObject(data) {
  $.checkArgument(data, 'data is required');
  var prevHash = data.prevHash;
  var merkleRoot = data.merkleRoot;
-  if (_.isString(data.prevHash)) {
+  if (typeof data.prevHash === 'string') {


This is false if data.prevHash instanceof String, whereas _.isString(new String('')) == true

kajoseph · 2022-05-02T17:43:30Z

packages/bitcore-lib/lib/crypto/bn.js

  return new BN(str, base);
 };

 BN.fromBuffer = function(buf, opts) {
-  if (typeof opts !== 'undefined' && opts.endian === 'little') {
+  if (opts !== undefined && opts.endian === 'little') {


We should probably make opts !== undefined be opts != null to avoid unnecessary confusion for a user passing in a null variable. There should be no difference between

BN.fromBuffer(buf);

and

var opts = null; BN.fromBuffer(buf, opts);

Same on line 89

Maybe consider changing all checks for undefined to checks for null as well (i.e. === undefined --> == null)

kajoseph · 2022-05-10T16:57:36Z

packages/bitcore-lib/lib/transaction/input/multisig.js

@@ -74,7 +60,7 @@ MultiSigInput.prototype.getSignatures = function(transaction, privateKey, index,

  var self = this;
  var results = [];
-  _.each(this.publicKeys, function(publicKey) {
+  this.publicKeys.forEach(function(publicKey) {


Is there a reason why you did .forEach instead a a for..of loop here?

kajoseph · 2022-05-10T17:00:59Z

packages/bitcore-lib/lib/transaction/input/multisigscripthash.js

@@ -122,7 +116,7 @@ MultiSigScriptHashInput.prototype.getSignatures = function(transaction, privateK

  var self = this;
  var results = [];
-  _.each(this.publicKeys, function(publicKey) {
+  this.publicKeys.forEach(function(publicKey) {


.forEach instead of for..of?

kajoseph · 2022-05-10T17:35:38Z

packages/bitcore-lib/lib/transaction/output.js

@@ -16,13 +15,13 @@ function Output(args) {
  if (!(this instanceof Output)) {
    return new Output(args);
  }
-  if (_.isObject(args)) {
+  if (typeof args === 'object') {


need to change all of these typeof ... === 'object' to use JSUtil.isObject

kajoseph · 2022-05-10T18:08:13Z

packages/bitcore-lib/lib/transaction/transaction.js

+  if (Array.isArray(privateKey)) {
+    privateKey.forEach(privateKey => this.sign(privateKey, sigtype, signingMethod));
+  } else {
+    this.getSignatures(privateKey, sigtype, signingMethod).forEach((signature) => {


forEach here and above

kajoseph · 2022-05-10T18:09:43Z

packages/bitcore-lib/lib/transaction/transaction.js

@@ -1222,40 +1207,34 @@ Transaction.prototype.applySignature = function(signature, signingMethod) {
 };

 Transaction.prototype.isFullySigned = function() {
-  _.each(this.inputs, function(input) {
+  this.inputs.forEach(function(input) {


kajoseph · 2022-05-10T18:15:34Z

packages/bitcore-lib/lib/util/js.js

+   * @description sort an array in place based on toString values
+   * @param {*[]} array
+   */
+  sort: array => array.sort((a, b) => a.toString() > b.toString() ? 1 : a.toString() < b.toString() ? -1 : 0),


It would probably be helpful to at least have a comment saying that this is an ascending sort. Might I suggest we have sortAsc and sortDesc functions? Just so we can avoid ambiguity.

kajoseph · 2022-05-10T18:28:34Z

packages/bitcore-lib/lib/util/js.js

+   * @param {*} obj 
+   * @returns {Boolean}
+   */
+  isObject: obj => typeof obj === 'object' && !!obj,


This is good. I think we should add similar functions for isNumber and isString and replace all the typeof <whatev>'s with them. What do you think? There are inconsistencies with checking typeof <xyz> === 'number' and typeof <xyz> === 'number' && !isNaN(<xyz>), plus the latter is a bit verbose.

Also, Number and String class instances would fail the typeof test, but would pass the lodash equivalents.

kajoseph · 2022-05-10T18:36:07Z

packages/bitcore-lib/test/transaction/transaction.js

+      transaction.outputs.should.include(out1);
+      transaction.outputs.should.include(out2);
+      transaction.outputs.should.include(out3);
+      transaction.outputs.should.include(out4);


This doesn't test for randomness anymore

kajoseph · 2022-05-11T20:44:58Z

The problem referenced in #3358 still applies to the .every method on the Array prototype. We should fix it in here

Sarah Bush added 2 commits March 18, 2022 18:17

substitute lodash calls in bitcore-lib

65d42f3

update addNetwork (lodash removal changes)

6f0a0ab

picnicbear force-pushed the bitcore-lib/removeLodash branch from 71cabec to 6f0a0ab Compare March 24, 2022 21:00

Sarah Bush added 9 commits March 25, 2022 09:56

fix invalid syntax from regex replacement

ed75389

update valid Base58.validCharacters to handle a string

9063482

add missing require for lodash, todo comments

36942f2

remove lodash from tests, convert forEaches to for loops, replace use…

3ee7b35

… of _.sortBy, fix multiple issues resulting from regex replacements

convert undefined checks to check equality instead of variable type, …

ac1d7b9

…convert more forEaches, fix a reduce

Merge branch 'master' of https://github.com/bitpay/bitcore into bitco…

ee5e904

…re-lib/removeLodash

add utils for sort, isObject, and shuffle to util/js, update more und…

4c62827

…efined type checks

replace dependencies on bitcore-lib's lodash outside of bitcore-lib

0262431

linting, replace lodash ref in bitcore p2p tests

0bdc2d2

kajoseph requested changes May 10, 2022

View reviewed changes

kajoseph mentioned this pull request May 11, 2022

String deriviation paths cause invalid hardening value due to Lodash call #3358

Open

twiztid89 approved these changes Nov 9, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Substitute lodash calls in bitcore-lib #3350

Substitute lodash calls in bitcore-lib #3350

picnicbear commented Mar 24, 2022

kajoseph left a comment

kajoseph May 2, 2022

kajoseph May 2, 2022

kajoseph May 10, 2022

kajoseph May 10, 2022

kajoseph May 10, 2022

kajoseph May 10, 2022

kajoseph May 10, 2022

kajoseph May 10, 2022

kajoseph May 10, 2022

kajoseph May 10, 2022

kajoseph commented May 11, 2022

Substitute lodash calls in bitcore-lib #3350

Are you sure you want to change the base?

Substitute lodash calls in bitcore-lib #3350

Conversation

picnicbear commented Mar 24, 2022

kajoseph left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kajoseph commented May 11, 2022